Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move extensions.LabelSelector to unversioned #20492

Merged
merged 3 commits into from
Feb 5, 2016
Merged

Move extensions.LabelSelector to unversioned #20492

merged 3 commits into from
Feb 5, 2016

Conversation

erictune
Copy link
Member

@erictune erictune commented Feb 2, 2016

Move type LabelSelector and type LabelSelectorRequirement from pkg/apis/extensions. This avoids an import loop when Job (and later DaemonSet, Deployment, ReplicaSet) are moved out of extensions to new api groups.

Also Move LabelSelectorAsSelector utility from pkg/apis/extensions/ to pkg/api/unversioned/. Also its test. Also LabelSelectorOp* constants. Also the pkg/apis/extensions/validation functions ValidateLabelSelectorRequirement and ValidateLabelSelector move to pkg/api/unversioned

The related type in pkg/apis/extensions/v1beta1/ is staying there. I might move
it in another PR if neccessary.

@erictune
Copy link
Member Author

erictune commented Feb 2, 2016

@kubernetes/goog-csi

@erictune
Copy link
Member Author

erictune commented Feb 2, 2016

Request quick review as I have like 5 PRs that stack on top of this that I need to get in this week.

@k8s-github-robot
Copy link

Labelling this PR as size/XL

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit fd6688b866208c81499ebf24ab3dcdf0552b4217.

@lavalamp lavalamp assigned lavalamp and unassigned bgrant0607 Feb 2, 2016
@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2016

I will look more closely momentarily, but I think @smarterclayton and I both want stuff like this to go into a "common" group, and avoid making the unversioned group contain more things.

@erictune
Copy link
Member Author

erictune commented Feb 2, 2016

Would you pretty please consider deferring that improvement until next week? You can even assign to me once you and clayton pick a name.
This took a long time to get right and I have like 5 more PRs I want to do that depend on this in the next 3 days.

@erictune
Copy link
Member Author

erictune commented Feb 2, 2016

Will fix: import loop with labelSelector #20462

@bgrant0607
Copy link
Member

cc @davidopp @madhusudancs

@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2016

Summary of IRL conversation: I asked for comments in the unversioned package to make it clear which types are serialized over the wire and which are merely shared internal code.

@davidopp
Copy link
Member

davidopp commented Feb 2, 2016

cc/ @kevin-wangzefeng

@erictune
Copy link
Member Author

erictune commented Feb 2, 2016

Added warning.

@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2016

LGTM, minor nits. self-apply the label when you fix, thanks.

@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit fa2dfe8073d49fb5b75a79da63de4fc62f9fca77.

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 2, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e test build/test passed for commit a5dad111e488a6da7e6c8caa09489950271346b9.

@lavalamp
Copy link
Member

lavalamp commented Feb 2, 2016

Need a pkg/apis/unversioned --> pkg/api/unversioned somewhere, unit tests don't build.

@k8s-bot
Copy link

k8s-bot commented Feb 2, 2016

GCE e2e build/test failed for commit 0faa06a5683295e501db68f52aecf5a2f7777dae.

@k8s-bot
Copy link

k8s-bot commented Feb 3, 2016

GCE e2e test build/test passed for commit fd52040a6697e9e967cc4050568ce5e2ee38d79d.

@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2016
@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 4, 2016
@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2016
@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 4, 2016

GCE e2e test build/test passed for commit c90a8e08b121e6f38c21749b97646e72a12e7947.

@k8s-bot
Copy link

k8s-bot commented Feb 4, 2016

GCE e2e test build/test passed for commit a5093bbd286d9c4aa9ed1c2cbd907c583f47e71b.

@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2016
@erictune
Copy link
Member Author

erictune commented Feb 4, 2016

@k8s-bot please test

@erictune
Move extensions.LabelSelector to unversioned.
6133cb1 
Move type LabelSelector and type LabelSelectorRequirement from pkg/apis/extensions
This avoids an import loop when Job (and later DaemonSet, Deployment, ReplicaSet)
are moved out of extensions to new api groups.

Also Move LabelSelectorAsSelector utility from pkg/apis/extensions/ to pkg/api/unversioned/
Also its test.
Also LabelSelectorOp* constants.
Also the pkg/apis/extensions/validation functions ValidateLabelSelectorRequirement and
ValidateLabelSelector move to pkg/api/unversioned

The related type in pkg/apis/extensions/v1beta1/ is staying there.  I might move
it in another PR if neccessary.
@erictune
Regenerate types, conversions.
fead374
@erictune
hack/update-generated-swagger-docs.sh
eba2c56
@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2016
@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 4, 2016

GCE e2e test build/test passed for commit eba2c56.

@k8s-github-robot
Copy link

Travis continuous integration appears to have missed, closing and re-opening to trigger it

lavalamp added a commit that referenced this pull request Feb 5, 2016
@lavalamp
Merge pull request #20492 from erictune/labselunver
d84ac76 
Move extensions.LabelSelector to unversioned
@lavalamp lavalamp merged commit d84ac76 into kubernetes:master Feb 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lavalamp lavalamp

Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@erictune @k8s-github-robot @k8s-bot @lavalamp @bgrant0607 @davidopp @googlebot